Looks like the tests failed, at least:

      ✕ should recover after exporting an invalid page (4063ms)
      ✕ should recover after a bad return from the render function (1153ms)

You can run them manually using:

yarn testonly --testPathPattern "basic" -t "should recover after exporting an invalid page"

I wonder if we can make this check development only, and only load react-is in that case to save bundle size

That sounds like a good idea. Does Next.js have an existing pattern to do that?

One way is to add a NODE_ENV check before the isValidElementType check, and rely on webpack's dead code elimination to omit the react-is module when it detects that its exports aren't used:

if (process.env.NODE_ENV === 'development' && !isValidElementType(Component)) {
  throw new Error(`The default export is not a React Component in page: "${pathname}"`)
}

I'm trying this out, adding this to rules in build/webpack.js:

{
  include: /react-is/,
  sideEffects: false,
},

But react-is still seems to be included in the production bundle in the hello-world example.

I don't have too much experience using webpack's tree shaking, so not sure if I'm missing something.

I guess something like this should work:

if (process.env.NODE_ENV === 'development') {
  const {isValidElementType} = require('react-is')
  if(!isValidElementType(Component)) {
	throw new Error(`The default export is not a React Component in page: "${pathname}"`)
  }
}

In general, I always wrap the process.env.NODE_ENV in an outer if so we're 100% sure uglify will remove the dead code.

Trying it out now, and it appears that react-is is still included in the production bundle even with the suggested outer if statement.

I've pushed my changes to this branch, and you can verify with:

npm install
npm link
cd examples/hello-world/
npm install
npm link next

# Build for production:
npm run build
npm start

Open http://localhost:3000 and look at the commons JS asset (mine is http://localhost:3000/_next/static/chunks/commons.792d7d9de2a7d11ba82f.js). You'll find the minified source of react-is in here.

I wonder if it’s shipped with React?

I’ll have a look tomorrow 👍🏻

I suspect that the reason why webpack/uglify's dead code elimination is not working is that Next.js's compiled sources use CJS modules instead of ES modules. e.g. in node_modules/next/dist/client/index.js:

var _reactIs = _interopRequireDefault(require("react-is"));

Not 100% sure though; I'll have more time to verify this weekend.

Uglify's dead code elimination works by removing blocks that have if(false) for example, webpack replaced process.env.NODE_ENV === 'development' compiles to 'development' === 'development' which is also removed by uglify

Fixed in 5ddd91c

I had to add IgnorePlugin for that specific file. Apparently webpack still bundles the require otherwise 🙏

Awesome, thanks 🙏

Looks like the tests are failing though.

Looks like the should recover after exporting an invalid page test is failing again, now due to the fact that the isValidElementType check doesn't occur on production:

  ● Basic Features › Error Recovery › should recover after exporting an invalid page

    expect(received).toMatch(expected)

    Expected value to match:
      /The default export is not a React Component/
    Received:
      "Element type is invalid: expected a string (for built-in components) or a class/function (for composite components) but got: object.

The test suite doesn't enter the if (process.env.NODE_ENV === 'development') {} block, so React's own error is raised instead upon render.

Should I update the test to expect React's error instead? Or maybe change the test suite so specific tests are executed using process.env.NODE_ENV === 'development'

Hello 😄This PR seems great 👍

Do you think we could consider having react-is in the production bundle if/when the tree-shakeable version is merged and released facebook/react#13321

Or is it better to keep react-is outside of the production bundle and we should rather change the tests ?

I'm going to close this PR as it has merge conflicts and the test was breaking, but I'll add good first issue / help wanted labels to the original issue and post that you already implement most here so that someone can take a look 💯